Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for SSL auth on api calls. #577

Merged
merged 6 commits into from
Apr 25, 2016

Conversation

dansajner
Copy link
Contributor

Addresses #399

@dansajner
Copy link
Contributor Author

I signed the CLA yesterday. Not sure why that check is failing.

@elasticsearch-release
Copy link

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run; then say 'jenkins, test it'.

@bastelfreak
Copy link
Member

@dansajner did you sign it with a mail address that isn't the one in your github profile or the one you used in the commit?

@dansajner
Copy link
Contributor Author

Looks like my my global git config has a different email address than the one I signed with, but what I signed with matches my github profile.

@dansajner
Copy link
Contributor Author

@bastelfreak do I need to push an update with a different email address? I pushed a fix to the tests so those should pass if run again.

@tylerjl
Copy link
Contributor

tylerjl commented Apr 6, 2016

Hey @dansajner, sorry for the delayed response here - this looks great, thanks for the PR. The CLA check looks good. 👍

Sorry for the inconvenience, but I've been updating some of the CI scripts for the repository and it's created a minor conflict so Travis can't test the PR. Could you update your branch with the latest from master and push again so it'll get picked up and tested?

From my perspective your changes look good, barring any flags from the test suite.

@tylerjl
Copy link
Contributor

tylerjl commented Apr 12, 2016

The intake tests are showing a failure when setting a variable to the empty string. I actually wasn't familiar with that check (results of inheriting the project), but after looking at the check's documentation I'm pretty sure this is a valid case for the empty string. According to the puppet lint docs we should be able to include something like the following surrounding the assignment to make the linter happy.

# lint:ignore:empty_string_assignment
$ssl_args = ''
# lint:endignore

@dansajner
Copy link
Contributor Author

@tylerjl all tests are passing.

@tylerjl tylerjl merged commit 4681dcf into voxpupuli:master Apr 25, 2016
@jarpy jarpy removed the in progress label Apr 25, 2016
@dansajner dansajner deleted the support_ssl branch April 25, 2016 15:06
cegeka-jenkins pushed a commit to cegeka/puppet-elasticsearch that referenced this pull request Oct 23, 2017
Add support for SSL auth on api calls.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants